PP-13312 Settings for card types get controller #4376

james-peacock-gds · 2024-12-05T12:46:50Z

Get controller for simplified settings / card types
As design stipulates that admin users should see checkboxes and non-admins should see a simple list, implement two methods for formatting the card types data and two child templates to slot into the parent card types page.
Retain existing logic to disable checkbox for Maestro if the service doesn't have 3ds enabled.
Update existing logic to provide hint to enable with Worldpay for American Express and UnionPay - this is only necessary if it's a Worldpay service.

For subsequent PRs:

implement the post controller
Cypress tests

Screenshot for admin user

Screenshot for non-admin user

DomBelcher · 2024-12-05T17:18:35Z

app/utils/simplified-account/format/format-card-types.js

+    if (['American Express', 'Union Pay'].includes(cardTypeChecklistItem.text)) {
+      if (account.paymentProvider === 'worldpay') {


this could be an if( A && B ) rather than nested if statements

oswaldquek · 2024-12-06T12:38:08Z

app/utils/simplified-account/format/format-card-types.test.js

+    requires3ds: true
+  }
+]
+describe('format-card-types for template', () => {


Testing for these hints in this test would be good too?

good point - this is now done.

oswaldquek · 2024-12-06T12:38:57Z

app/utils/simplified-account/format/format-card-types.js

+    return {
+      ...cardTypeChecklistItem,
+      disabled: true,
+      hint: {


How are these hints shown in the UI? For example, when the mouse hovers over the label?

They are always displayed as additional text underneath the card type if the conditions are met. The text under 'Maestro' in the first screenshot is an example of this.

oswaldquek · 2024-12-06T13:23:58Z

Nice 1

Use ControllerTestBuilder

nlsteers

few 💬

nlsteers · 2024-12-06T13:28:12Z

app/controllers/simplified-account/settings/card-types/card-types.controller.test.js

+  })
+}
+
+const setupTest = (method, user, additionalReqProps = {}, additionalStubs = {}) => {


can you refactor this test to use the new ControllerTestBuilder?

nlsteers · 2024-12-06T13:29:15Z

app/services/clients/connector.client.js

@@ -257,6 +257,21 @@ ConnectorClient.prototype = {
    return response.data
  },

+  /**
+   * Retrieves the accepted card Types for the given account


service external id and account type is a more appropriate description to help differentiate from old gateway account based endpoints

nlsteers · 2024-12-06T13:30:32Z

app/utils/simplified-account/format/format-card-types.js

+}
+
+const addHintForAmexAndUnionpayIfWorldpay = (cardTypeChecklistItem, paymentProvider) => {
+  if (['American Express', 'Union Pay'].includes(cardTypeChecklistItem.text) && paymentProvider === 'worldpay') {


we should put these in an object somewhere

I've declared the array at the top of the file

nlsteers · 2024-12-06T13:38:12Z

app/utils/simplified-account/format/format-card-types.test.js

+]
+describe('format-card-types for template', () => {
+  describe('present checkboxes for admin user', () => {
+    it('should return all card types with checked boxes if they are all accepted', () => {


I think we can improve these tests, something like:

const expectedDebitCards = ['Visa debit', 'Mastercard debit', 'Maestro'] const expectedCreditCards = ['Visa credit', 'American Express', 'JCB'] expect(cards.debitCards).to.have.length(expectedDebitCards.length) expect(cards.creditCards).to.have.length(expectedCreditCards.length) cards.debitCards.forEach((card, idx) => { expect(card).to.deep.include({ text: expectedDebitCards[idx], checked: true }) }) cards.creditCards.forEach((card, idx) => { expect(card).to.deep.include({ text: expectedCreditCards[idx], checked: true }) })

agree that using to.deep.include reduces repetition so I've updated this test and the next one. I'm not sure that using iterable brings a significant benefit with this small number of items and as far as I can see it isn't something that can be reused elsewhere in these tests. Happy to go with it though if you prefer.

DomBelcher · 2024-12-10T09:58:00Z

app/utils/simplified-account/format/format-card-types.js

+  return { debitCards: debitCardChecklistItems, creditCards: creditCardChecklistItems }
+}
+
+const formatCardTypesForNonAdminTemplate = (allCards, acceptedCards) => {


the best I could come up with for refactoring this is something like this, but I'm not convinced it's an improvement so feel free to ignore it and stick with this implementation

Suggested change

const formatCardTypesForNonAdminTemplate = (allCards, acceptedCards) => {

const isDisabled = (card, account) => {

return card.requires3ds && !account.requires3ds

}

const hintText = (card, account) => {

const cardLabel = formatLabel(card)

const disabled = isDisabled(card, account)

if (disabled && account.type === 'test') {

return {

text: `${cardLabel} is not available on test accounts`

}

} else if (disabled) {

return {

text: `${cardLabel} cannot be used because 3D Secure is not available. Please contact support`

}

}

if (card.type !== 'CREDIT' || account.paymentProvider !== 'worldpay') {

return undefined

}

if (['American Express', 'Union Pay'].includes(cardLabel)) {

return {

html: 'You must have already enabled this with Worldpay'

}

}

return undefined

}

const formatCard = (card, accepted, account) => {

const disabled = isDisabled(card, account)

return {

value: card.id,

text: formatLabel(card),

checked: !disabled && accepted,

requires3ds: card.requires3ds,

disabled,

hint: hintText(card, account)

}

}

const formatCardTypesForNonAdminTemplate = (allCards, acceptedCards) => {

const items = allCards.reduce((acc, card) => {

if (!acc[card.type]) {

acc[card.type] = []

}

const accepted = acceptedCards.filter(c => c.type === card.type && c.id === card.id).length > 0

acc[card.type].push(formatCard(card, accepted, account))

return acc

}, {})

return {

debitCards: Object.values(items.DEBIT),

creditCards: Object.values(items.CREDIT)

}

}

DomBelcher · 2024-12-10T10:14:02Z

app/utils/simplified-account/format/format-card-types.js

+  const formattedCardTypes = {
+    'Enabled debit cards': [],
+    'Not enabled debit cards': [],
+    'Enabled credit cards': [],
+    'Not enabled credit cards': []
+  }
+  allCards.forEach(card => {
+    const cardIsEnabled = acceptedCardTypeIds.includes(card.id) ? 'Enabled' : 'Not enabled'
+    formattedCardTypes[`${cardIsEnabled} ${card.type.toLowerCase()} cards`].push(formatLabel(card))
+  })
+  return formattedCardTypes
+}


a slight change I might suggest would be something like

Suggested change

const formattedCardTypes = {

'Enabled debit cards': [],

'Not enabled debit cards': [],

'Enabled credit cards': [],

'Not enabled credit cards': []

}

allCards.forEach(card => {

const cardIsEnabled = acceptedCardTypeIds.includes(card.id) ? 'Enabled' : 'Not enabled'

formattedCardTypes[`${cardIsEnabled} ${card.type.toLowerCase()} cards`].push(formatLabel(card))

})

return formattedCardTypes

}

const formattedCardTypes = {

'debit/enabled': {

cards: [],

heading: 'Enabled debit cards'

}

...

}

allCards.forEach(card => {

const cardIsEnabled = acceptedCardTypeIds.includes(card.id) ? 'enabled' : 'disabled'

formattedCardTypes[`${card.type.toLowerCase()}/${cardIsEnabled}`].push(formatLabel(card))

})

just so the heading isn't the object key

james-peacock-gds force-pushed the PP-13312-Settings_card_types branch from efd06b5 to 3e8966c Compare December 5, 2024 12:53

PP-13312 Get controller for card types

62a37a9

james-peacock-gds force-pushed the PP-13312-Settings_card_types branch from 25e1099 to 62a37a9 Compare December 5, 2024 13:02

PP-13312 Fix test

988891a

DomBelcher reviewed Dec 5, 2024

View reviewed changes

james-peacock-gds marked this pull request as draft December 6, 2024 10:49

PP-13312 [squash] refactor format-card-types

9c770fd

james-peacock-gds marked this pull request as ready for review December 6, 2024 11:33

oswaldquek reviewed Dec 6, 2024

View reviewed changes

PP-13312 [squash] update tests

5d4726b

oswaldquek previously approved these changes Dec 6, 2024

View reviewed changes

nlsteers requested changes Dec 6, 2024

View reviewed changes

james-peacock-gds added 3 commits December 6, 2024 14:32

PP-13312 response to comments

5435fac

PP-13312 [squash] improve tests as recommended

0f30819

PP-13312 [squash] add test for nonadmin

576877a

james-peacock-gds force-pushed the PP-13312-Settings_card_types branch from 25a26cd to 576877a Compare December 6, 2024 15:37

DomBelcher reviewed Dec 10, 2024

View reviewed changes

nlsteers approved these changes Dec 10, 2024

View reviewed changes

james-peacock-gds merged commit a8e47a0 into master Dec 10, 2024
14 checks passed

james-peacock-gds deleted the PP-13312-Settings_card_types branch December 10, 2024 15:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PP-13312 Settings for card types get controller #4376

PP-13312 Settings for card types get controller #4376

james-peacock-gds commented Dec 5, 2024 •

edited

Loading

DomBelcher Dec 5, 2024

james-peacock-gds Dec 6, 2024

oswaldquek Dec 6, 2024

james-peacock-gds Dec 6, 2024

oswaldquek Dec 6, 2024

james-peacock-gds Dec 6, 2024

oswaldquek commented Dec 6, 2024

nlsteers left a comment

nlsteers Dec 6, 2024

james-peacock-gds Dec 6, 2024

nlsteers Dec 6, 2024

james-peacock-gds Dec 6, 2024

nlsteers Dec 6, 2024

james-peacock-gds Dec 6, 2024

nlsteers Dec 6, 2024

james-peacock-gds Dec 6, 2024

DomBelcher Dec 10, 2024

DomBelcher Dec 10, 2024

		if (['American Express', 'Union Pay'].includes(cardTypeChecklistItem.text)) {
		if (account.paymentProvider === 'worldpay') {

-const formatCardTypesForNonAdminTemplate = (allCards, acceptedCards) => {
+const isDisabled = (card, account) => {
+  return card.requires3ds && !account.requires3ds
+}
+const hintText = (card, account) => {
+  const cardLabel = formatLabel(card)
+  const disabled = isDisabled(card, account)
+  if (disabled && account.type === 'test') {
+    return {
+      text: `${cardLabel} is not available on test accounts`
+    }
+  } else if (disabled) {
+    return {
+      text: `${cardLabel} cannot be used because 3D Secure is not available. Please contact support`
+    }
+  }
+  if (card.type !== 'CREDIT' || account.paymentProvider !== 'worldpay') {
+    return undefined
+  }
+  if (['American Express', 'Union Pay'].includes(cardLabel)) {
+    return {
+      html: 'You must have already enabled this with Worldpay'
+    }
+  }
+  return undefined
+}
+const formatCard = (card, accepted, account) => {
+  const disabled = isDisabled(card, account)
+  return {
+    value: card.id,
+    text: formatLabel(card),
+    checked: !disabled && accepted,
+    requires3ds: card.requires3ds,
+    disabled,
+    hint: hintText(card, account)
+  }
+}
+const formatCardTypesForNonAdminTemplate = (allCards, acceptedCards) => {
+  const items = allCards.reduce((acc, card) => {
+    if (!acc[card.type]) {
+      acc[card.type] = []
+    }
+    const accepted = acceptedCards.filter(c => c.type === card.type && c.id === card.id).length > 0
+    acc[card.type].push(formatCard(card, accepted, account))
+    return acc
+  }, {})
+  return {
+    debitCards: Object.values(items.DEBIT),
+    creditCards: Object.values(items.CREDIT)
+  }
+}

PP-13312 Settings for card types get controller #4376

PP-13312 Settings for card types get controller #4376

Conversation

james-peacock-gds commented Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oswaldquek commented Dec 6, 2024

nlsteers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

james-peacock-gds commented Dec 5, 2024 •

edited

Loading